Repository navigation
feat(cli): record which config key failed in config-load telemetry (CLI-2912) - #7468
Conversation
…LI-2912) Failed cli_command_executed events classified invalid_config now carry config_key, the config key path behind the failure. Keys come from a closed vocabulary built from the config schema and registry, so values, file paths and user-chosen names never reach telemetry.
…erry/cli-2912-record-which-config-key-failed-in-config-load-telemetry # Conflicts: # apps/cli/src/shared/telemetry/event-catalog.ts # apps/cli/src/telemetry/command-telemetry-attributes.ts
There was a problem hiding this comment.
🤖 AI Review
Verified all three findings against the checked-out code and trusted conventions. After merging the overlapping stale-key reports, two minor telemetry attribution issues remain confirmed.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/src/config/cli-config-values.layer.ts:71 |
telemetry-accuracy |
claude+codex | A recovered config failure can leave its config_key attached to a later, unrelated invalid_config failure. Keyless failures do not clear the previously recorded key, and the final telemetry gate checks only the error category. |
| 🟡 MINOR | apps/cli/src/config/cli-config-failure-key.ts:62 |
telemetry-accuracy |
codex | An unselected remote named after a root config section can cause config_key to identify the remote name instead of the failing section. For example, a remote named auth with db = "invalid" reports auth instead of db. |
Stats
Claude findings: 1 · Codex findings: 2 · Confirmed: 2 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
CLI benchmark (Lightweight Commands): no changes foundCommit
API calls: unchanged for all 7 commands. Binary size: 134.1 MB, up 12.0 KB from 134.0 MB. Details: timing ranges, API calls per command, slowest phasesTiming ranges
API calls per command
Slowest phasesNo command moved beyond noise, so there are no phase deltas. Comment |
…ovenance A keyless config failure or a successful load or lookup now clears the recorded key, so a recovered failure cannot label a later one. Remote-block schema failures are detected from the decode itself instead of by comparing prefix depth, so a remote named after a config section reports the failing section.
A successful load or lookup with no earlier failure no longer records an empty config_key, so commands that assert their recorded attributes see only their own.
Summary
A failed
cli_command_executedevent shows that a command failed on config (error_category: invalid_config,error_fingerprint: tag:StartConfigLoadError), but not which key caused it. So we can't tell how many config-load failures come from the removed[inbucket]section and how many from everything else.This adds a
config_keyproperty to those events.CliConfigValues.loadandResolvedCliConfig.getrecord the key through the existingrecordCommandTelemetrysink when they fail. It has to be recorded at load time because command wrappers such asStartConfigLoadErrorkeep only the message and drop the original errorCliConfigRemovedKeyError: every removed key, sorted and comma-joinedCliConfigValueError: its pathCliConfigParseError: the first issue path, when the cause is a schema errorfunctions.my-fn.verify_jwtrecordsfunctionsand a customauth.external.<name>recordsauth.externalwithCommandTelemetryonly addsconfig_keywhen the event'serror_categoryisinvalid_config, so a config failure that a command swallowed can't label a success or an unrelated failureapps/cli/docs/analytics.mddocuments the new propertyThe list of removed keys is kept in the CLI rather than exported from
@supabase/config, so this change stays out of that package's release train.Linked issue
Closes CLI-2912
🤖 Generated with Claude Code